feat: replace snapshotId with name-based sandbox persistence#114
Open
recoup-coding-agent wants to merge 1 commit intomainfrom
Open
feat: replace snapshotId with name-based sandbox persistence#114recoup-coding-agent wants to merge 1 commit intomainfrom
recoup-coding-agent wants to merge 1 commit intomainfrom
Conversation
With Vercel's new sandbox names feature, sandbox state persists
automatically by name. This removes explicit snapshot-taking and
snapshotId passing throughout the tasks codebase.
- Remove snapshot() calls from codingAgentTask, updatePRTask, setupSandboxTask
- Remove snapshotId from schemas, callbacks, and return types
- Use Sandbox.get({ name }) instead of Sandbox.get({ sandboxId })
- snapshotAndPersist now only persists github_repo metadata
- Update all related tests
Co-Authored-By: Paperclip <noreply@paperclip.ing>
📝 WalkthroughWalkthroughThis PR deprecates snapshot management across the codebase by removing Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/sandboxes/getOrCreateSandbox.ts (1)
30-35:⚠️ Potential issue | 🔴 Critical
Sandbox.get()requiressandboxIdparameter, notname.The official Vercel
@vercel/sandboxSDK documentation confirms the signature isSandbox.get({ sandboxId: string; ... }). The current code passesname: created.sandboxIdwhich will fail at runtime. The tests pass because they use mocked implementations that don't validate parameter names, but the real SDK will reject this parameter.This same issue affects
src/tasks/updatePRTask.tsandsrc/tasks/runSandboxCommandTask.ts.Suggested fix
const sandbox = await Sandbox.get({ - name: created.sandboxId, + sandboxId: created.sandboxId, token, teamId, projectId, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/sandboxes/getOrCreateSandbox.ts` around lines 30 - 35, The call to Sandbox.get uses the wrong parameter key; change the object passed to Sandbox.get in getOrCreateSandbox (replace name: created.sandboxId with sandboxId: created.sandboxId) and make the same fix in the other affected call sites referenced in the review (updatePRTask.ts and runSandboxCommandTask.ts), ensuring every invocation uses Sandbox.get({ sandboxId: <id>, token, teamId, projectId }) so it matches the SDK signature.
🧹 Nitpick comments (2)
src/tasks/setupSandboxTask.ts (1)
8-12: Stale JSDoc comment references snapshot behavior.The comment still mentions "takes a snapshot" but the task no longer takes snapshots—it only persists metadata.
📝 Proposed fix
/** * Background task that creates a personal Vercel Sandbox for an account, - * fully provisions it (OpenClaw, GitHub repo, org repos, folder structure), - * takes a snapshot, and shuts it down. + * fully provisions it (OpenClaw, GitHub repo, org repos, folder structure), + * persists metadata, and shuts it down. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tasks/setupSandboxTask.ts` around lines 8 - 12, Update the stale JSDoc in setupSandboxTask.ts: remove the "takes a snapshot" wording and change the description to state that the task persists sandbox metadata rather than capturing a snapshot. Locate the top-of-file comment for the background task (setupSandboxTask) and replace the snapshot phrase with a brief note that it "persists sandbox metadata and shuts it down" so the doc matches the current behavior.src/tasks/updatePRTask.ts (1)
24-37: Consider usinggetOrCreateSandboxfor consistency.Other tasks like
codingAgentTaskandsetupSandboxTaskusegetOrCreateSandboxwhich encapsulates sandbox credential retrieval and provides a consistent pattern. This task bypasses that abstraction, directly callingSandbox.get.If the sandbox name concern above is resolved, consider whether this inconsistency is intentional or if using
getOrCreateSandbox(or a similar reusable pattern) would be better.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tasks/updatePRTask.ts` around lines 24 - 37, The code in run (updatePRTask) directly calls Sandbox.get with manual credentials (using getVercelSandboxCredentials and CODING_AGENT_ACCOUNT_ID) instead of the shared helper; replace that block to call getOrCreateSandbox (the same helper used by codingAgentTask and setupSandboxTask), remove the manual getVercelSandboxCredentials usage, and use the sandbox returned by getOrCreateSandbox for logging and later use; update references from Sandbox.get(...) to getOrCreateSandbox(...) so credential handling and sandbox creation/resumption are consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/recoup/updateAccountSnapshot.ts`:
- Around line 12-16: The updateAccountSnapshot function currently drops the
optional _snapshotId and so no longer clears persisted snapshot_id in the
backend; modify updateAccountSnapshot (and any call sites that pass _snapshotId)
to explicitly send a clear value (e.g., null or empty-string per API contract)
when the intent is to remove the saved snapshot_id so the backend restores do
not keep stale state; ensure the request body includes snapshot_id: null (or "")
rather than omitting the field, and coordinate with
src/recoup/getAccountSandboxes (which still models snapshot_id) and
src/sandboxes/getOrCreateSandbox (which expects POST /api/sandboxes to restore)
to preserve the intended migration semantics.
In `@src/tasks/updatePRTask.ts`:
- Around line 30-35: The code in updatePRTask uses CODING_AGENT_ACCOUNT_ID as
the sandbox name when calling Sandbox.get, but createAccountSandbox returns an
API-generated sandboxId; update updatePRTask to use the actual sandbox ID
returned/stored by getOrCreateSandbox/createAccountSandbox instead of
CODING_AGENT_ACCOUNT_ID — locate the resume logic in updatePRTask where
Sandbox.get({... name: CODING_AGENT_ACCOUNT_ID ...}) is called and replace it
with the stored sandboxId (or call getOrCreateSandbox and pass its
created.sandboxId) so Sandbox.get receives the API-generated sandbox name;
ensure any initialization that persists the sandbox ID (from
createAccountSandbox or getOrCreateSandbox) is read before calling Sandbox.get.
---
Outside diff comments:
In `@src/sandboxes/getOrCreateSandbox.ts`:
- Around line 30-35: The call to Sandbox.get uses the wrong parameter key;
change the object passed to Sandbox.get in getOrCreateSandbox (replace name:
created.sandboxId with sandboxId: created.sandboxId) and make the same fix in
the other affected call sites referenced in the review (updatePRTask.ts and
runSandboxCommandTask.ts), ensuring every invocation uses Sandbox.get({
sandboxId: <id>, token, teamId, projectId }) so it matches the SDK signature.
---
Nitpick comments:
In `@src/tasks/setupSandboxTask.ts`:
- Around line 8-12: Update the stale JSDoc in setupSandboxTask.ts: remove the
"takes a snapshot" wording and change the description to state that the task
persists sandbox metadata rather than capturing a snapshot. Locate the
top-of-file comment for the background task (setupSandboxTask) and replace the
snapshot phrase with a brief note that it "persists sandbox metadata and shuts
it down" so the doc matches the current behavior.
In `@src/tasks/updatePRTask.ts`:
- Around line 24-37: The code in run (updatePRTask) directly calls Sandbox.get
with manual credentials (using getVercelSandboxCredentials and
CODING_AGENT_ACCOUNT_ID) instead of the shared helper; replace that block to
call getOrCreateSandbox (the same helper used by codingAgentTask and
setupSandboxTask), remove the manual getVercelSandboxCredentials usage, and use
the sandbox returned by getOrCreateSandbox for logging and later use; update
references from Sandbox.get(...) to getOrCreateSandbox(...) so credential
handling and sandbox creation/resumption are consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8904621-6cec-43b1-bd16-34c52ec09ed5
📒 Files selected for processing (18)
src/recoup/updateAccountSnapshot.tssrc/sandboxes/__tests__/getOrCreateSandbox.test.tssrc/sandboxes/__tests__/notifyCodingAgentCallback.test.tssrc/sandboxes/__tests__/snapshotAndPersist.test.tssrc/sandboxes/ensureGithubRepo.tssrc/sandboxes/getOrCreateSandbox.tssrc/sandboxes/notifyCodingAgentCallback.tssrc/sandboxes/snapshotAndPersist.tssrc/schemas/codingAgentSchema.tssrc/schemas/sandboxSchema.tssrc/schemas/updatePRSchema.tssrc/tasks/__tests__/codingAgentTask.test.tssrc/tasks/__tests__/setupSandboxTask.test.tssrc/tasks/__tests__/updatePRTask.test.tssrc/tasks/codingAgentTask.tssrc/tasks/runSandboxCommandTask.tssrc/tasks/setupSandboxTask.tssrc/tasks/updatePRTask.ts
💤 Files with no reviewable changes (5)
- src/sandboxes/tests/notifyCodingAgentCallback.test.ts
- src/schemas/codingAgentSchema.ts
- src/schemas/updatePRSchema.ts
- src/sandboxes/notifyCodingAgentCallback.ts
- src/schemas/sandboxSchema.ts
Comment on lines
12
to
+16
| export async function updateAccountSnapshot( | ||
| accountId: string, | ||
| snapshotId?: string, | ||
| _snapshotId?: string, | ||
| githubRepo?: string | ||
| ): Promise<{ success: boolean; snapshotId: string } | undefined> { | ||
| ): Promise<{ success: boolean } | undefined> { |
There was a problem hiding this comment.
This no longer clears persisted snapshot_id state.
src/recoup/getAccountSandboxes.ts:5-9 still models snapshot_id, and src/sandboxes/getOrCreateSandbox.ts:12-13 says POST /api/sandboxes still performs restoration internally. Deprecating _snapshotId without sending an explicit clear value means old snapshot links can survive this rollout, which makes the name-based migration incomplete and can keep the backend restoring stale sandbox state.
Also applies to: 22-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/recoup/updateAccountSnapshot.ts` around lines 12 - 16, The
updateAccountSnapshot function currently drops the optional _snapshotId and so
no longer clears persisted snapshot_id in the backend; modify
updateAccountSnapshot (and any call sites that pass _snapshotId) to explicitly
send a clear value (e.g., null or empty-string per API contract) when the intent
is to remove the saved snapshot_id so the backend restores do not keep stale
state; ensure the request body includes snapshot_id: null (or "") rather than
omitting the field, and coordinate with src/recoup/getAccountSandboxes (which
still models snapshot_id) and src/sandboxes/getOrCreateSandbox (which expects
POST /api/sandboxes to restore) to preserve the intended migration semantics.
Comment on lines
+30
to
35
| const sandbox = await Sandbox.get({ | ||
| name: CODING_AGENT_ACCOUNT_ID, | ||
| token, | ||
| teamId, | ||
| projectId, | ||
| source: { type: "snapshot", snapshotId }, | ||
| timeout: 30 * 60 * 1000, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check what createAccountSandbox returns and how sandbox names are determined
# Find createAccountSandbox implementation
echo "=== createAccountSandbox implementation ==="
ast-grep --pattern 'async function createAccountSandbox($_) {
$$$
}'
# Also check with export syntax
ast-grep --pattern 'export async function createAccountSandbox($_) {
$$$
}'
# Search for createAccountSandbox to find its definition
echo ""
echo "=== Searching for createAccountSandbox definition ==="
rg -n "createAccountSandbox" -A 20 --type ts
# Check if sandbox name uses accountId directly anywhere
echo ""
echo "=== Check Sandbox.get/create name parameter patterns ==="
rg -n "Sandbox\.(get|create)" -A 5 --type tsRepository: recoupable/tasks
Length of output: 10841
Fix sandbox name mismatch: use actual sandbox ID instead of account ID.
updatePRTask attempts to resume the sandbox using CODING_AGENT_ACCOUNT_ID directly as the sandbox name. However, createAccountSandbox() returns an API-generated sandboxId (e.g., "sbx_123"), which is passed to Sandbox.get({ name: created.sandboxId }) in getOrCreateSandbox.
The constant CODING_AGENT_ACCOUNT_ID is a UUID that will never match the actual sandbox name created by the API. This will cause Sandbox.get() to fail at runtime, preventing updatePRTask from connecting to the sandbox.
To fix: Pass the sandbox ID returned from createAccountSandbox() to Sandbox.get(), or update the initialization logic to store and retrieve the actual sandbox ID before updatePRTask attempts to resume it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tasks/updatePRTask.ts` around lines 30 - 35, The code in updatePRTask
uses CODING_AGENT_ACCOUNT_ID as the sandbox name when calling Sandbox.get, but
createAccountSandbox returns an API-generated sandboxId; update updatePRTask to
use the actual sandbox ID returned/stored by
getOrCreateSandbox/createAccountSandbox instead of CODING_AGENT_ACCOUNT_ID —
locate the resume logic in updatePRTask where Sandbox.get({... name:
CODING_AGENT_ACCOUNT_ID ...}) is called and replace it with the stored sandboxId
(or call getOrCreateSandbox and pass its created.sandboxId) so Sandbox.get
receives the API-generated sandbox name; ensure any initialization that persists
the sandbox ID (from createAccountSandbox or getOrCreateSandbox) is read before
calling Sandbox.get.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sandbox.snapshot()calls with Vercel's name-based sandbox persistencesnapshotIdfrom task schemas, callback payloads, and return typesSandbox.get({ name })instead ofSandbox.get({ sandboxId })throughoutsnapshotAndPersistnow only persistsgithub_repometadata (no more snapshot-taking)Part of REC-36. Companion PR in
apirepo handles the API-side changes.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
snapshotIdfrom callbacks and API responses.